Skip to content

harden(distributed): defensive companion-artifact option handling (follow-ups to #11075)#11094

Closed
localai-bot wants to merge 2 commits into
masterfrom
harden/distributed-companion-options
Closed

harden(distributed): defensive companion-artifact option handling (follow-ups to #11075)#11094
localai-bot wants to merge 2 commits into
masterfrom
harden/distributed-companion-options

Conversation

@localai-bot

Copy link
Copy Markdown
Collaborator

What this is (and is not)

Two small defensive-hardening follow-ups to #11075 for managed companion artifacts (a model that pulls a second HuggingFace repo, e.g. longcat-video-avatar-1.5's base_model).

This is NOT a fix for a live bug. The remote-load failure that motivated the investigation (base_model must point to a LongCat-Video checkpoint, backend re-downloading the base repo) turned out to be a stale longcat-video worker backend that predated #10949 and lacked _resolve_option_path; upgrading the backend fixed it, verified end-to-end with a real video on a clean cluster (released controller image, no workarounds). These changes just make two adjacent code paths fail-safe instead of silently dropping a companion.

1. Unresolved companion → declared source repo (not silence)

withCompanionArtifactOptions used to emit nothing for a declared companion that had no resolved snapshot. A dropped companion is invisible to the backend, which then loads its own hardcoded default and fails far from the cause. Now it falls back to the companion's declared source repository id, so the backend fetches the artifact the config actually asked for. The resolved snapshot path (the staged, no-download fast path) is still preferred whenever the companion is resolved — the healthy path is unchanged — and the fallback logs a warning so a missing controller-side resolution stays diagnosable.

2. Reconciler replay re-derives companion options

The reconciler's scale-up path (ScheduleAndLoadModel) replays a ModelOptions proto blob captured at first load and never runs grpcModelOpts. A companion resolved after that blob was captured would never reach a scaled-up replica. applyCompanionOptions re-derives the model's managed companion options from its current config (new exported backend.CompanionArtifactOptions, wired via SmartRouterOptions.CompanionOptionsFor in distributed startup) and merges in only those the blob is missing — an option the blob already carries (including an author pin) always wins. nil-safe: single-node and any caller that doesn't wire the resolver are an unchanged no-op.

Also adds a regression guard: FileStagingClient (the client wrapping every distributed LoadModel) must forward ModelOptions.Options and ModelPath to the backend unchanged, so a future refactor can't silently drop options.

Tests (Ginkgo/Gomega, TDD)

  • core/backend/companion_options_test.go — unresolved companion names the source repo; resolved companion still prefers the snapshot path.
  • core/services/nodes/router_companion_replay_test.go — reconciler injects a companion missing from the stored blob; never overrides one already present; no-op without a resolver. (Red-before / green-after confirmed by stubbing applyCompanionOptions.)
  • core/services/nodes/file_staging_loadmodel_test.go — the handoff forwards .Options/ModelPath unchanged.

Verification (exit codes read separately)

  • go build ./core/... ./pkg/... → 0
  • go test ./core/backend/ → 0
  • go test ./core/config/ → 0
  • go test ./core/application/ → 0
  • go test ./core/services/nodes/ → 0 (~200s)
  • golangci-lint run --new-from-merge-base=origin/master (core/backend, core/config, core/services/nodes, core/application) → 0 issues
  • tests/e2e grep for the touched symbols → no references.

No diagnostic tracing and no backend changes ship in this PR.

🤖 Generated with Claude Code

https://claude.ai/code/session_0156CbKpDh8qbAYzJZJDZ2yk

mudler added 2 commits July 23, 2026 21:25
…esolved

Defensive follow-up to #11075. It does NOT fix the longcat-video remote-load
failure observed earlier — that turned out to be a stale worker backend
(predating #10949, so it lacked _resolve_option_path) and was fixed by
upgrading the backend. This only hardens the option-synthesis path.

withCompanionArtifactOptions previously dropped a declared companion artifact
whenever it had no resolved snapshot, emitting nothing. A dropped companion is
invisible to the backend, which then falls back to its own hardcoded default and
fails far from the cause. Instead, fall back to the companion's DECLARED source
repository id so the backend fetches the artifact the config actually asked for;
the resolved snapshot path (the staged, no-download fast path) is still
preferred whenever the companion is resolved, so the healthy path is unchanged.
The fallback logs a warning so the missing controller-side resolution stays
diagnosable.

The synthesis is factored into an exported ManagedCompanionOptions /
CompanionArtifactOptions so the distributed reconciler can re-derive the same
options when it replays a stored proto blob (next commit).

Assisted-by: Claude Code:claude-opus-4-8[1m] [Read] [Edit] [Bash]

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…replays a stored blob

Defensive follow-up to #11075; not the fix for the earlier longcat-video remote
failure (that was a stale worker backend, fixed by upgrading it).

The reconciler's scale-up path (ScheduleAndLoadModel) replays a ModelOptions
proto blob captured at first load and never runs grpcModelOpts. So a companion
resolved AFTER that blob was captured (e.g. the blob predates the companion's
resolution) would never reach a scaled-up replica, which would then load without
the companion option. applyCompanionOptions re-derives the model's managed
companion options from its CURRENT config (via the new
backend.CompanionArtifactOptions, wired through SmartRouterOptions.CompanionOptionsFor
in distributed startup) and merges in only those the blob is missing — an option
the blob already carries, including an author pin, always wins. nil-safe: single
node and any caller that does not wire the resolver are an unchanged no-op.

Also adds a regression guard for the controller->worker handoff:
FileStagingClient (the client wrapping every distributed LoadModel) must forward
ModelOptions.Options — the channel a managed companion option rides on — and
ModelPath to the backend unchanged. The test pins that so a future refactor of
the wrapper cannot silently drop options.

Assisted-by: Claude Code:claude-opus-4-8[1m] [Read] [Edit] [Bash]

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
@mudler mudler closed this Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants